feat(web-next): implement remote follow UIRemote follow#214
feat(web-next): implement remote follow UIRemote follow#214dahlia merged 20 commits intohackers-pub:mainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show remote follow dialog for unauthenticated viewers, and support onFollowed callback for post-follow navigation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Handles OStatus subscribe redirects, shows actor card with follow button, and navigates to profile on success. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (7)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.Add a configuration file to your project to customize how CodeRabbit runs oxc. 📝 WalkthroughWalkthroughAdds WebFinger-based remote-follow support: new GraphQL WebFingerResult and lookup resolver, web-next RemoteFollowButton UI and authorize_interaction route, Viewer authentication context and root wiring, FollowButton adjustments, and i18n entries across locales. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RemoteFollowButton
participant GraphQL_API
participant WebFinger_Resolver
participant Nodeinfo_Service
User->>RemoteFollowButton: Submit Fediverse handle
RemoteFollowButton->>GraphQL_API: lookupRemoteFollower(followerHandle, actorId)
GraphQL_API->>WebFinger_Resolver: resolve webfinger (acct:user@host)
WebFinger_Resolver->>WebFinger_Resolver: normalize & validate handle
WebFinger_Resolver->>WebFinger_Resolver: fetch actor (ActivityPub)
WebFinger_Resolver->>Nodeinfo_Service: fetch nodeinfo (software)
Nodeinfo_Service-->>WebFinger_Resolver: nodeinfo response
WebFinger_Resolver->>WebFinger_Resolver: extract icons, emojis, remoteFollowUrl
WebFinger_Resolver-->>GraphQL_API: WebFingerResult
GraphQL_API-->>RemoteFollowButton: actor info
User->>RemoteFollowButton: Confirm remote follow
RemoteFollowButton->>User: Open remoteFollowUrl (new tab)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's interoperability with the Fediverse by integrating remote follow capabilities. It provides a seamless experience for users to follow accounts hosted on other instances, whether they are authenticated or not, by introducing a new GraphQL API for WebFinger lookups, dedicated UI components for interaction, and a robust page to manage the authorization flow. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request implements the remote follow functionality for the web-next stack, which is a great addition for federation. The changes include a new GraphQL query for WebFinger lookups, a RemoteFollowButton component, and a new page to handle OStatus subscribe redirects. The overall implementation is solid, but I've identified a couple of critical Cross-Site Scripting (XSS) vulnerabilities where unsanitized remote data is rendered as HTML. I've also suggested a minor improvement for URL encoding on the backend. Please address the security issues with high priority.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
web-next/src/contexts/ViewerContext.tsx (1)
9-11: Use a named props interface forViewerProvider.The component works, but defining props inline here diverges from the repo’s TS props convention and reduces reuse/readability.
As per coding guidelines "Use interfaces for component props (e.g., ButtonProps) in TypeScript".♻️ Proposed refactor
+interface ViewerProviderProps { + isAuthenticated: () => boolean; +} + -export const ViewerProvider: ParentComponent<{ - isAuthenticated: () => boolean; -}> = (props) => { +export const ViewerProvider: ParentComponent<ViewerProviderProps> = (props) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-next/src/contexts/ViewerContext.tsx` around lines 9 - 11, Replace the inline props type on ViewerProvider with a named interface: create an exported interface (e.g., ViewerProviderProps) that declares isAuthenticated: () => boolean, then update the component signature to ParentComponent<ViewerProviderProps> and use that interface where the props type is referenced (i.e., the ViewerProvider function definition and any related type imports/exports) to follow the repo TS props convention and improve reuse/readability.graphql/schema.graphql (1)
1377-1383: Use stronger scalar types for URL-shaped fields inWebFingerResult.
iconUrlandurlare URL values but currently typed asString, which weakens schema validation and client contract clarity.♻️ Proposed schema refinement
type WebFingerResult { domain: String emojis: JSON handle: String - iconUrl: String + iconUrl: URL name: String preferredUsername: String remoteFollowUrl: String software: String summary: String - url: String + url: URL }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@graphql/schema.graphql` around lines 1377 - 1383, The WebFingerResult type uses plain String for URL-shaped fields; change the iconUrl and url field types to a URL scalar (e.g., replace "iconUrl: String" and "url: String" with "iconUrl: URL" and "url: URL"), ensure the URL scalar is declared/imported in the schema (or add a GraphQL scalar type named URL that validates URLs), and update any resolvers/type mappings that construct WebFingerResult (functions or classes that return iconUrl or url) to return/validate a proper URL value or string that passes the URL scalar validation.web-next/src/components/RemoteFollowButton.tsx (1)
63-65: Destructure component props at the function boundary.This keeps the component aligned with repo conventions and simplifies access across handlers.
As per coding guidelines "Files with components use PascalCase naming (e.g., Button.tsx) Use functional components with props destructuring".♻️ Proposed refactor
-export function RemoteFollowButton(props: RemoteFollowButtonProps) { +export function RemoteFollowButton( + { actorHandle, actorName }: RemoteFollowButtonProps, +) { @@ - actorHandle: props.actorHandle, + actorHandle, @@ - const displayName = () => props.actorName || props.actorHandle; + const displayName = () => actorName || actorHandle;Also applies to: 115-116, 144-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-next/src/components/RemoteFollowButton.tsx` around lines 63 - 65, The component currently receives props via a single identifier; change RemoteFollowButton to destructure its RemoteFollowButtonProps at the function boundary (e.g., function RemoteFollowButton({ ... }: RemoteFollowButtonProps)) so handlers can use the props directly, update any internal references that access props.* to the new local names, and apply the same props-destructuring refactor to the other component declarations in this file that follow the same pattern (the other component functions that currently accept a props identifier).graphql/webfinger.ts (1)
121-125: Add an explicit return type tolookupWebFingerImpl.This function returns a complex object-or-null shape; please annotate it explicitly (ideally via a shared result type used by both
buildWebFingerResultand fallback returns).As per coding guidelines:
**/*.{ts,tsx}: Use explicit typing for complex return types in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@graphql/webfinger.ts` around lines 121 - 125, Add an explicit TypeScript return type for lookupWebFingerImpl that describes the complex object-or-null shape it returns; define a shared interface or type alias (e.g., WebFingerResult or IWebFingerResult) that matches the return shape produced by buildWebFingerResult and use that type on lookupWebFingerImpl's signature and on any fallback return values (including null if appropriate) so both buildWebFingerResult and lookupWebFingerImpl reference the same result type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@graphql/webfinger.ts`:
- Around line 148-155: remoteFollowLink.template is untrusted; before building
remoteFollowUrl ensure the template contains the literal "{uri}", replace using
encodeURIComponent(actorHandle) (not encodeURI), then parse the resulting URL
with the URL API and only accept it if its protocol is "http:" or "https:"; if
the template is missing "{uri}" or the parsed URL has a non-http(s) scheme or
fails to parse, do not return remoteFollowUrl (leave undefined) and skip
including the link in responses. Apply this validation around the
remoteFollowLink -> remoteFollowUrl logic (symbols: webfingerResult.links,
remoteFollowLink, remoteFollowUrl, actorHandle).
In `@web-next/src/components/RemoteFollowButton.tsx`:
- Around line 149-150: The fallback username parsing using
info.handle?.split("@")[0] can return an empty string for handles like
"@user@host"; update the fallback so it first strips leading "@" characters from
info.handle (e.g. remove any leading @ symbols), then split on "@" and take the
first non-empty segment, and keep the existing fallbacks info.name and
info.preferredUsername; modify the expression that currently uses
info.handle?.split("@")[0] (in RemoteFollowButton.tsx) to perform this sanitized
extraction and return the sanitized result or "" if nothing valid remains.
- Around line 230-233: The h4 in RemoteFollowButton.tsx is using innerHTML with
actorDisplayName(), which injects remote metadata and opens an XSS vector;
change it to render the display name as plain text (remove
innerHTML/dangerouslySetInnerHTML and pass actorDisplayName() as the element's
children or use textContent), and ensure the actorDisplayName() helper returns a
plain string (no HTML) or is sanitized before rendering.
- Around line 135-141: Validate info.remoteFollowUrl before opening it: in
RemoteFollowButton.tsx, replace the direct window.open call with a guard that
constructs a URL object from info.remoteFollowUrl (inside a try/catch) and
checks that url.protocol is "http:" or "https:"; if invalid or construction
fails call setError(t`This service does not support remote follow.`) and return,
otherwise call window.open(url.toString(), "_blank", "noopener,noreferrer") and
then handleOpenChange(false). Ensure you reference info.remoteFollowUrl,
setError, and handleOpenChange in the fix.
In `@web-next/src/routes/`(root)/authorize_interaction.tsx:
- Around line 72-80: The code always calls loadPageQuery("") via
createPreloadedQuery even when uri() is falsy; change it to avoid invoking
loadPageQuery when there's no URI by only calling createPreloadedQuery (and thus
loadPageQuery) if uri() returns a truthy value—e.g., compute const u = uri(); if
(!u) set data to null/undefined and let the component render the “No user URI
provided.” fallback; otherwise call
createPreloadedQuery(authorizeInteractionPageQuery, () => loadPageQuery(u)).
Update any downstream logic that reads data to handle the null/undefined case.
- Around line 23-35: The identifier authorizeInteractionPageQuery is declared
twice (once in the type import and once as the GraphQL document constant),
causing a redeclare lint error; rename one of them (e.g., alias the imported
type to AuthorizeInteractionPageQueryType or rename the const to
authorizeInteractionPageDocument) and update any usages accordingly—look for the
import line importing authorizeInteractionPageQuery and the const declaration
using graphql as well as related usages in preload, loadPageQuery, and any route
typing to ensure the new name is used consistently.
---
Nitpick comments:
In `@graphql/schema.graphql`:
- Around line 1377-1383: The WebFingerResult type uses plain String for
URL-shaped fields; change the iconUrl and url field types to a URL scalar (e.g.,
replace "iconUrl: String" and "url: String" with "iconUrl: URL" and "url: URL"),
ensure the URL scalar is declared/imported in the schema (or add a GraphQL
scalar type named URL that validates URLs), and update any resolvers/type
mappings that construct WebFingerResult (functions or classes that return
iconUrl or url) to return/validate a proper URL value or string that passes the
URL scalar validation.
In `@graphql/webfinger.ts`:
- Around line 121-125: Add an explicit TypeScript return type for
lookupWebFingerImpl that describes the complex object-or-null shape it returns;
define a shared interface or type alias (e.g., WebFingerResult or
IWebFingerResult) that matches the return shape produced by buildWebFingerResult
and use that type on lookupWebFingerImpl's signature and on any fallback return
values (including null if appropriate) so both buildWebFingerResult and
lookupWebFingerImpl reference the same result type.
In `@web-next/src/components/RemoteFollowButton.tsx`:
- Around line 63-65: The component currently receives props via a single
identifier; change RemoteFollowButton to destructure its RemoteFollowButtonProps
at the function boundary (e.g., function RemoteFollowButton({ ... }:
RemoteFollowButtonProps)) so handlers can use the props directly, update any
internal references that access props.* to the new local names, and apply the
same props-destructuring refactor to the other component declarations in this
file that follow the same pattern (the other component functions that currently
accept a props identifier).
In `@web-next/src/contexts/ViewerContext.tsx`:
- Around line 9-11: Replace the inline props type on ViewerProvider with a named
interface: create an exported interface (e.g., ViewerProviderProps) that
declares isAuthenticated: () => boolean, then update the component signature to
ParentComponent<ViewerProviderProps> and use that interface where the props type
is referenced (i.e., the ViewerProvider function definition and any related type
imports/exports) to follow the repo TS props convention and improve
reuse/readability.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
graphql/mod.tsgraphql/schema.graphqlgraphql/webfinger.tsweb-next/src/components/FollowButton.tsxweb-next/src/components/RemoteFollowButton.tsxweb-next/src/contexts/ViewerContext.tsxweb-next/src/locales/en-US/messages.poweb-next/src/locales/ja-JP/messages.poweb-next/src/locales/ko-KR/messages.poweb-next/src/locales/zh-CN/messages.poweb-next/src/locales/zh-TW/messages.poweb-next/src/routes/(root).tsxweb-next/src/routes/(root)/authorize_interaction.tsx
There was a problem hiding this comment.
Pull request overview
Adds “remote follow” support to the web-next Solid/Relay frontend and exposes a new GraphQL WebFinger lookup to power OStatus subscribe redirects (issue #173).
Changes:
- Introduces
lookupWebFingerGraphQL query +WebFingerResulttype for WebFinger/ActivityPub lookup and remote-follow URL discovery. - Adds viewer auth state via
ViewerContext, and updatesFollowButtonto show aRemoteFollowButtonfallback when unauthenticated. - Adds
/authorize_interactionroute to support inbound OStatus-style follow authorization flow and post-follow navigation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| web-next/src/routes/(root)/authorize_interaction.tsx | New authorize-interaction page for inbound remote-follow redirects and follow confirmation UI |
| web-next/src/routes/(root).tsx | Wraps app in ViewerProvider to expose auth state globally |
| web-next/src/contexts/ViewerContext.tsx | Adds viewer auth context (isAuthenticated) |
| web-next/src/components/FollowButton.tsx | Adds unauthenticated remote-follow fallback + onFollowed callback |
| web-next/src/components/RemoteFollowButton.tsx | New dialog flow to look up WebFinger + open remote instance follow URL |
| graphql/webfinger.ts | New GraphQL resolver to perform WebFinger + ActivityPub lookup and build remoteFollowUrl |
| graphql/schema.graphql | Adds lookupWebFinger and WebFingerResult to schema |
| graphql/mod.ts | Registers the new webfinger.ts module |
| web-next/src/locales/en-US/messages.po | Adds translations for remote follow/authorize interaction strings |
| web-next/src/locales/ja-JP/messages.po | Adds translations for remote follow/authorize interaction strings |
| web-next/src/locales/ko-KR/messages.po | Adds translations for remote follow/authorize interaction strings |
| web-next/src/locales/zh-CN/messages.po | Adds translations for remote follow/authorize interaction strings |
| web-next/src/locales/zh-TW/messages.po | Adds translations for remote follow/authorize interaction strings |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web-next/src/locales/zh-TW/messages.po (1)
1546-1549: Translation adds content not present in source string.The source text
"You are about to follow {0}."is translated as"你即將從你的帳戶關注{0}。"which back-translates to "You are about to follow {0} from your account." The phrase "從你的帳戶" (from your account) was added.While this may provide helpful context in the UI, it deviates from the source string. Consider aligning with the source:
Suggested fix
#. placeholder {0}: actor().name ?? actor().handle #: src/routes/(root)/authorize_interaction.tsx:127 msgid "You are about to follow {0}." -msgstr "你即將從你的帳戶關注{0}。" +msgstr "你即將關注{0}。"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-next/src/locales/zh-TW/messages.po` around lines 1546 - 1549, The translated string msgstr for msgid "You are about to follow {0}." adds extra content ("從你的帳戶") not present in the source; update the msgstr to closely match the source by removing that added phrase so it reads a direct equivalent of "You are about to follow {0}." (e.g., "你即將關注{0}。"), preserving the placeholder and punctuation used in the original msgid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@graphql/webfinger.ts`:
- Around line 76-99: A parsing error on one tag currently escapes the whole
try/catch around the actorObject.getTags() loop and aborts emoji extraction;
update the loop that iterates actorObject.getTags() so each iteration is guarded
with its own try/catch (wrap the body that checks "tag instanceof vocab.Emoji",
calls tag.getIcon(), reads emojiIcon.url, constructs new URL(raw), and assigns
emojis[emojiName]) and on exception continue to the next tag (optionally log the
error); ensure you still skip non-Emoji tags and preserve the existing
validation checks for emojiIcon.url and protocol before assigning into the
emojis map.
---
Nitpick comments:
In `@web-next/src/locales/zh-TW/messages.po`:
- Around line 1546-1549: The translated string msgstr for msgid "You are about
to follow {0}." adds extra content ("從你的帳戶") not present in the source; update
the msgstr to closely match the source by removing that added phrase so it reads
a direct equivalent of "You are about to follow {0}." (e.g., "你即將關注{0}。"),
preserving the placeholder and punctuation used in the original msgid.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
graphql/webfinger.tsweb-next/src/components/RemoteFollowButton.tsxweb-next/src/locales/en-US/messages.poweb-next/src/locales/ja-JP/messages.poweb-next/src/locales/ko-KR/messages.poweb-next/src/locales/zh-CN/messages.poweb-next/src/locales/zh-TW/messages.poweb-next/src/routes/(root)/authorize_interaction.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web-next/src/locales/ko-KR/messages.po
- web-next/src/components/RemoteFollowButton.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
graphql/webfinger.ts (1)
91-97:⚠️ Potential issue | 🟡 MinorOne malformed emoji URL can abort all emoji extraction.
The
new URL(raw)on line 91 is not guarded by its own try-catch. If any single emoji has a malformed URL, the exception propagates to the outer catch block (line 100), terminating the loop and skipping all remaining emojis.🛠️ Proposed fix
const emojiName = tag.name.toString(); const raw = emojiIcon.url instanceof vocab.Link ? emojiIcon.url.href!.href : emojiIcon.url.href; - const u = new URL(raw); - if ( - (u.protocol === "http:" || u.protocol === "https:") && - !/[\'\"]/.test(raw) - ) { - emojis[emojiName] = u.href; + try { + const u = new URL(raw); + if ( + (u.protocol === "http:" || u.protocol === "https:") && + !/[\'\"]/.test(raw) + ) { + emojis[emojiName] = u.href; + } + } catch { + continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@graphql/webfinger.ts` around lines 91 - 97, The loop that builds the emojis map uses new URL(raw) unguarded, so a single malformed raw will throw and abort processing; wrap the URL construction/validation for each emoji (the new URL(raw) call inside the block that assigns emojis[emojiName]) in its own try/catch (or pre-validate the string) and only assign emojis[emojiName] = u.href when URL parsing succeeds and the protocol check passes, ensuring a malformed emoji URL is skipped without breaking the outer loop or aborting the whole extraction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@graphql/webfinger.ts`:
- Around line 91-97: The loop that builds the emojis map uses new URL(raw)
unguarded, so a single malformed raw will throw and abort processing; wrap the
URL construction/validation for each emoji (the new URL(raw) call inside the
block that assigns emojis[emojiName]) in its own try/catch (or pre-validate the
string) and only assign emojis[emojiName] = u.href when URL parsing succeeds and
the protocol check passes, ensuring a malformed emoji URL is skipped without
breaking the outer loop or aborting the whole extraction.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
graphql/webfinger.tsweb-next/src/components/RemoteFollowButton.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web-next/src/components/RemoteFollowButton.tsx
dahlia
left a comment
There was a problem hiding this comment.
Thanks for implementing remote follow — the overall flow and UX are well thought out. I have some suggestions around the GraphQL query design, security, and a few smaller issues.
A single malformed emoji URL could abort the entire emoji extraction loop. Wrap per-emoji processing in its own try/catch so one bad tag doesn't prevent extracting the rest. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…query
Extract AuthorizeInteractionContent so createPreloadedQuery is only
called when a valid URI exists, preventing a wasteful loadPageQuery("")
call when the uri search param is absent.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
While auth query is loading, isAuthenticated() returns false, briefly showing RemoteFollowButton to authenticated users. Add isLoaded state and gate FollowButton rendering on it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract WebFingerResultData interface shared by buildWebFingerResult and lookupWebFingerImpl. Change iconUrl, url, remoteFollowUrl fields from String to URL scalar for stronger schema validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The msgstr for "You are about to follow {0}." contained "從你的帳戶"
(from your account) which is not present in the source string.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename fediverseId → followerHandle and actorHandle → actorId (Relay global ID) so the server can validate it refers to a real Hackers' Pub actor. Add GraphQL descriptions to the query and its parameters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Require rel="self" and exact-match the link type against
application/activity+json or the standard LD-JSON ActivityStreams
profile, instead of the overly broad includes("activity") check.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (4)
web-next/src/routes/(root)/authorize_interaction.tsx (2)
27-30:⚠️ Potential issue | 🟡 MinorNormalize
acct:URIs before callingactorByHandle.If
uriarrives asacct:user@host, this goes straight intoactorByHandle, causing an avoidable failed lookup path and extra latency.🔧 Proposed fix
export const route = { preload(args) { - const uri = new URLSearchParams(args.location.search).get("uri") ?? ""; - if (uri) { - void loadPageQuery(uri); + const rawUri = new URLSearchParams(args.location.search).get("uri") ?? ""; + const uri = rawUri.replace(/^acct:/i, ""); + if (uri) { + void loadPageQuery(uri); } }, } satisfies RouteDefinition; @@ - const uri = () => searchParams.uri as string | undefined; + const uri = () => (searchParams.uri as string | undefined)?.replace(/^acct:/i, "");Also applies to: 39-39, 69-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-next/src/routes/`(root)/authorize_interaction.tsx around lines 27 - 30, The code passes raw "acct:" URIs into actorByHandle causing failed lookups; before calling loadPageQuery and anywhere actorByHandle is invoked (e.g., where uri or handle is sourced), detect and normalize strings starting with "acct:" by stripping the "acct:" prefix (and URL-decoding/trimming) to produce "user@host" or the expected handle format, then pass the normalized handle into loadPageQuery/actorByHandle so lookups use the correct handle format and avoid the extra failed path.
23-35:⚠️ Potential issue | 🔴 CriticalFix duplicate
authorizeInteractionPageQueryidentifier (lint blocker).The type import and GraphQL document constant share the same name, which triggers
noRedeclareand blocks lint/check.🔧 Proposed fix
-import type { authorizeInteractionPageQuery } from "./__generated__/authorizeInteractionPageQuery.graphql.ts"; +import type { + authorizeInteractionPageQuery as AuthorizeInteractionPageQuery, +} from "./__generated__/authorizeInteractionPageQuery.graphql.ts"; @@ -const authorizeInteractionPageQuery = graphql` +const authorizeInteractionPageQueryDocument = graphql` query authorizeInteractionPageQuery($uri: String!) { @@ const loadPageQuery = query( (uri: string) => - loadQuery<authorizeInteractionPageQuery>( + loadQuery<AuthorizeInteractionPageQuery>( useRelayEnvironment()(), - authorizeInteractionPageQuery, + authorizeInteractionPageQueryDocument, { uri }, ), @@ - const data = createPreloadedQuery<authorizeInteractionPageQuery>( - authorizeInteractionPageQuery, + const data = createPreloadedQuery<AuthorizeInteractionPageQuery>( + authorizeInteractionPageQueryDocument, () => loadPageQuery(props.uri), );#!/bin/bash set -euo pipefail file="$(fd -p 'authorize_interaction.tsx' web-next/src/routes | head -n1)" rg -nP '\bauthorizeInteractionPageQuery\b' "$file" sed -n '20,40p' "$file"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-next/src/routes/`(root)/authorize_interaction.tsx around lines 23 - 35, The file declares a type import authorizeInteractionPageQuery and also a const graphql document with the same name, causing a redeclare lint error; fix by renaming one of them (e.g., alias the type import: import type { authorizeInteractionPageQuery as authorizeInteractionPageQueryType } or rename the graphql const to authorizeInteractionPageQueryDocument) and update any local references to the renamed symbol (check the import line and the const declaration for authorizeInteractionPageQuery and adjust usages accordingly).graphql/webfinger.ts (2)
227-276:⚠️ Potential issue | 🟠 MajorAdd abuse controls for unauthenticated remote-follow lookups.
This resolver can trigger multiple outbound requests per call and is publicly reachable, so it should be rate-limited (or similarly throttled) to reduce abuse and scanning risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@graphql/webfinger.ts` around lines 227 - 276, The public resolver lookupRemoteFollower (its resolve function) can be abused to trigger many outbound requests; add throttling by checking an authentication/context rate limit before calling lookupRemoteFollowerImpl. Specifically, in the resolve for lookupRemoteFollower, use a per-client key (e.g., ctx.ip or ctx.auth?.userId or ctx.apiKey) and invoke the existing rate limiter (e.g., ctx.rateLimiter.consume or a new token-bucket) to allow only a small number of calls per time window; if the limit is exceeded, log the event and return null/early error instead of calling lookupRemoteFollowerImpl. Keep the validation of args.followerHandle and actor lookup intact and apply the limiter before the outbound lookup to protect lookupRemoteFollower and lookupRemoteFollowerImpl from abuse.
158-163:⚠️ Potential issue | 🟠 MajorHarden ActivityPub link selection and validate URL scheme before lookup.
Current matching is too permissive (
includes("activity")), andhrefis used for network fetch without explicithttp/httpsvalidation.🔧 Proposed hardening
- const activityPubLink = webfingerResult.links?.find((link) => - link.type === "application/activity+json" || - (link.rel === "self" && link.type?.includes("activity")) - ) as WebfingerLink | undefined; + const activityPubLink = webfingerResult.links?.find((link) => + link.rel === "self" && + ( + link.type === "application/activity+json" || + link.type === 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' + ) + ) as WebfingerLink | undefined; if (!activityPubLink?.href) return null; + let activityPubHref: string; + try { + const parsed = new URL(activityPubLink.href); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return null; + activityPubHref = parsed.href; + } catch { + return null; + } @@ - const actorObject = await ctx.fedCtx.lookupObject(activityPubLink.href, { + const actorObject = await ctx.fedCtx.lookupObject(activityPubHref, { documentLoader, }); @@ - url: new URL(activityPubLink.href), + url: new URL(activityPubHref),#!/bin/bash set -euo pipefail file="$(fd -p 'webfinger.ts' graphql | head -n1)" rg -nP 'includes\\("activity"\\)|lookupObject\\(activityPubLink\\.href' "$file" sed -n '150,200p' "$file"Also applies to: 190-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@graphql/webfinger.ts` around lines 158 - 163, The ActivityPub link selection is too permissive and uses href for network calls without validating the URL scheme; tighten the matching and reject non-http(s) hrefs before calling lookupObject. Update the link find logic over webfingerResult.links to only accept link.type === "application/activity+json" or (link.rel === "self" && link.type?.startsWith("application/activity+json")) (or other precise MIME strings you expect), then after selecting activityPubLink validate its href by parsing with the URL constructor and ensuring url.protocol is "http:" or "https:"; if parsing fails or the protocol is not http/https return null instead of passing the href to lookupObject(activityPubLink.href). Use the activityPubLink variable name and the lookupObject call as the places to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@graphql/webfinger.ts`:
- Around line 227-276: The public resolver lookupRemoteFollower (its resolve
function) can be abused to trigger many outbound requests; add throttling by
checking an authentication/context rate limit before calling
lookupRemoteFollowerImpl. Specifically, in the resolve for lookupRemoteFollower,
use a per-client key (e.g., ctx.ip or ctx.auth?.userId or ctx.apiKey) and invoke
the existing rate limiter (e.g., ctx.rateLimiter.consume or a new token-bucket)
to allow only a small number of calls per time window; if the limit is exceeded,
log the event and return null/early error instead of calling
lookupRemoteFollowerImpl. Keep the validation of args.followerHandle and actor
lookup intact and apply the limiter before the outbound lookup to protect
lookupRemoteFollower and lookupRemoteFollowerImpl from abuse.
- Around line 158-163: The ActivityPub link selection is too permissive and uses
href for network calls without validating the URL scheme; tighten the matching
and reject non-http(s) hrefs before calling lookupObject. Update the link find
logic over webfingerResult.links to only accept link.type ===
"application/activity+json" or (link.rel === "self" &&
link.type?.startsWith("application/activity+json")) (or other precise MIME
strings you expect), then after selecting activityPubLink validate its href by
parsing with the URL constructor and ensuring url.protocol is "http:" or
"https:"; if parsing fails or the protocol is not http/https return null instead
of passing the href to lookupObject(activityPubLink.href). Use the
activityPubLink variable name and the lookupObject call as the places to modify.
In `@web-next/src/routes/`(root)/authorize_interaction.tsx:
- Around line 27-30: The code passes raw "acct:" URIs into actorByHandle causing
failed lookups; before calling loadPageQuery and anywhere actorByHandle is
invoked (e.g., where uri or handle is sourced), detect and normalize strings
starting with "acct:" by stripping the "acct:" prefix (and
URL-decoding/trimming) to produce "user@host" or the expected handle format,
then pass the normalized handle into loadPageQuery/actorByHandle so lookups use
the correct handle format and avoid the extra failed path.
- Around line 23-35: The file declares a type import
authorizeInteractionPageQuery and also a const graphql document with the same
name, causing a redeclare lint error; fix by renaming one of them (e.g., alias
the type import: import type { authorizeInteractionPageQuery as
authorizeInteractionPageQueryType } or rename the graphql const to
authorizeInteractionPageQueryDocument) and update any local references to the
renamed symbol (check the import line and the const declaration for
authorizeInteractionPageQuery and adjust usages accordingly).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
graphql/schema.graphqlgraphql/webfinger.tsweb-next/src/components/FollowButton.tsxweb-next/src/components/RemoteFollowButton.tsxweb-next/src/contexts/ViewerContext.tsxweb-next/src/locales/zh-TW/messages.poweb-next/src/routes/(root).tsxweb-next/src/routes/(root)/authorize_interaction.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web-next/src/locales/zh-TW/messages.po
|
Could you run |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Implements remote follow functionality for the web-next stack (closes #173).
lookupWebFingerGraphQL query for WebFinger + OStatus subscribe template lookupViewerContextto provide auth state across the appRemoteFollowButtoncomponent with Fediverse ID input dialog and actor previewFollowButtonto show remote follow fallback for unauthenticated viewers, and supportonFollowedcallback/authorize_interactionpage for OStatus subscribe redirects with post-follow navigationFlow
@user@mastodon.social)/authorize_interaction?uri=..., authenticates the user and shows a follow confirmation cardValidation
deno task checkSummary by CodeRabbit
New Features
Enhancements
Localization